Repository navigation
[5.x] Let asset_folders_list filter to root folders - #20464
Open
brandonkelly wants to merge 2 commits into
Open
brandonkelly wants to merge 2 commits into
brandonkelly wants to merge 2 commits into
Conversation
criteria.parentId now accepts null, which lists volumes' root folders. Their IDs are needed to list a volume's top-level folders or create new ones with asset_folders_create. Folder criteria ignore a null parentId, so it's mapped to :empty:. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-c93e4e62-8dbe-4153-b265-a57d6fb57ac8
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regression test bypasses MCP schema validation, so it would miss a regression that rejects null parentId values and restores the client invalid-parameters error.
1 open issue
What changed in this PR
This PR lets MCP clients query volume root folders with asset_folders_list, providing the parent IDs needed to list or create top-level folders.
Changes:
- Accepts
nullforcriteria.parentIdand maps it to the explicit root-folder filter. - Adds a regression test confirming that root-folder queries exclude subfolders.
- Documents the behavior in the tool schema and changelog.
| File | Description |
|---|---|
tests/unit/mcp/AssetFoldersCapabilityTest.php |
Tests root-folder filtering with a null parent ID. |
src/mcp/capabilities/AssetFolders.php |
Allows null parent IDs and translates them into root-folder queries. |
CHANGELOG.md |
Announces support for querying root folders. |
Review effort: Balanced
| 'uid' => ['type' => 'string', 'format' => 'uuid'], | ||
| 'volumeId' => ['type' => 'integer'], | ||
| 'parentId' => ['type' => 'integer', 'description' => 'Parent folder ID.'], | ||
| 'parentId' => ['type' => ['integer', 'null'], 'description' => 'Parent folder ID, or null for volumes’ root folders. List a volume’s top-level folders with its root folder’s ID.'], |
Member
Author
There was a problem hiding this comment.
Fixed in 52b6968: the test now calls asset_folders_list through tools/call on the authenticated server, so the input schema applies. It fails both when parentId reverts to integer-only (-32602 … Expected 'integer', but received 'null') and when the :empty: mapping is removed (the subfolder is included).
Calling AssetFolders::list() directly bypassed the tool's input schema, so the test wouldn't catch parentId reverting to integer-only. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-c93e4e62-8dbe-4153-b265-a57d6fb57ac8
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
asset_folders_listcouldn't find a volume's root folder by query, becausecriteria.parentIdonly accepted integers. Agents need that folder's ID to list a volume's top-level folders, or to create one withasset_folders_create, which requiresparentId.FolderCriteriaignores a nullparentId, sofindFolders()would have returned every folder.nullis therefore mapped to:empty:, the same match thatAssets::getRootFolderByVolumeId()uses:Combine it with
volumeIdto get one volume's root folder. Then pass that folder's ID asparentIdto list the volume's top-level folders. TheparentIddescription says this.Craft 6:
craftcms/cms6.xhas the same integer-onlyparentIdinsrc/Mcp/Capabilities/AssetFolders.php, so this should be ported.Evidence
asset_folders_list {criteria: {parentId: null}}→Invalid parameters for tool 'asset_folders_list': Property '/criteria/parentId': Invalid type. Expected 'integer', but received 'null'.After: it returns only root folders 1000 and 1001 from the volume folder fixtures. The subfolder under 1000 is left out.
New
AssetFoldersCapabilityTest::testListFiltersToRootFoldersForNullParentIdcalls the tool throughtools/callon the authenticated server, so the input schema applies:-32602 Invalid parameters … Expected 'integer', but received 'null'Before (without the
:empty:mapping):Failed asserting that two arrays are equal.The subfolder was included.After:
OK (1 test, 2 assertions)ECS and Rector are clean. PHPStan passes (pre-commit hook).